-
Notifications
You must be signed in to change notification settings - Fork 10.1k
D1 REST API latency changelog #22751
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:
|
|
@lambrospetrou @joshthoward can you review? We do need to quantify the performance improvement. Doesn't have to be a graph, even just a ballpark % range |
|
Preview URL: https://c1b02344.preview.developers.cloudflare.com Files with changes (up to 15) |
|
|
||
| D1 query endpoints like `/query` and `/raw` have the most noticeable improvements since they no longer access Cloudflare's core data centers. D1 control plane endpoints such as those to create and delete databases see smaller improvements, since they still require access to Cloudflare's core data centers for other control plane metadata. | ||
|
|
||
| Performance improvements apply to only the REST API. Users can now expect similar performance querying their database via the REST API and via a [Worker](/d1/best-practices/query-d1/#query-d1-with-workers-binding-api). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also thought about mentioning comparison to the Worker binding API but they are still not quite the same. The REST API still does extra things, auth itself, we do some other internal calls, and so there is some overhead still until they are the same.
I would remove that sentence.
| Performance improvements apply to only the REST API. Users can now expect similar performance querying their database via the REST API and via a [Worker](/d1/best-practices/query-d1/#query-d1-with-workers-binding-api). | |
| These performance improvements only apply to the REST API. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lambrospetrou Are there any plans to address the REST overhead? Is the overhead 10s of ms (if so I could still frame as comparable performance which is the goal)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's 100-300ms more than the Worker Binding depending on caching of downstream services, so not negligible in some cases. I don't want folks to start complaining that they don't see same numbers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's 100-300ms more than the Worker Binding depending on caching of downstream services
is this specific to D1 or general REST overhead for CF APi?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it's not general CF API. Well, auth is, but the rest is for other things we check in D1 (e.g. entitlements, feature flags, and others) only during REST API requests. We can reduce that further later with some followups.
Co-authored-by: Lambros Petrou <[email protected]>
* D1 REST API latency changelog --------- Co-authored-by: Lambros Petrou <[email protected]>
No description provided.